Skip to content

Dev proxy for Worker-owned paths in plain vp dev + font-build gitignore - #170

Merged
audreyt merged 7 commits into
mainfrom
ship/2026-08-17-heading-layout
Sep 15, 2026
Merged

audreyt merged 7 commits into
mainfrom
ship/2026-08-17-heading-layout

Conversation

@audreyt

@audreyt audreyt commented Aug 17, 2026 •

Copy link
Copy Markdown
Member

原本的版面修改(wrapping-flex 單列)已由 #173 的 narrow rail 設計取代,rebase 時四個版面檔(src/index.css、src/pages/DictionaryPage.tsx、tests/e2e/dictionary.spec.ts、tests/e2e/visual-invariants.spec.ts)直接取 main 版,AGENTS.md 的版型 bullet 也改寫為 narrow-rail 描述。本 PR 剩下 main 還沒有的部分:

  • vite.config.ts:plain vp dev(無 Worker)時把 Worker 擁有的路徑(/api、/assets)proxy 到 VITE_DEV_API_ORIGIN,public/ 真實檔案由 bypass 優先本地服務。含 Copilot review 指出的 hardening:tryDecodeURIComponent 解碼、單次 statSync try/catch(無 existsSync TOCTOU)、new URL 守衛。
  • tests/unit/dev-proxy-public-bypass.test.ts:上述契約測試(壞編碼回 false 不丟例外、遍歷拒絕、真實檔案優先)。
  • .gitignore:repo-root fonts/ 構建產物忽略+已刪除的 issue-triage 否定清理。
  • AGENTS.md:本機 dev 兩種模式文件。

驗證:tsc -b --noEmit clean;dev-proxy 5 tests、offline-api 11 tests 通過;版面四檔與 main byte-identical(沿用 main CI 的 e2e)。

Closes the conflict reported in #170 (comment).

Copilot AI lite review requested due to automatic review settings August 17, 2026 06:19
Mobile heading controls dropped to their own row unconditionally: the icon
row was pinned to `grid-row: 3` and `.radical` was squeezed into a
`max-width: 4.5rem` column, so /萌 at 375px burned four stacked rows
(艸 / +8 / = 12 / pencil, then copy·異體字·star) with obvious free space
next to the headword.

Root blocker was `.entry-copy-status { min-width: 8rem }`: a permanent
inline reservation for the copy feedback. 8rem plus three 44px targets can
never fit beside a title at phone widths, so the icon row could not share a
line no matter how much room the title left.

- `.entry-heading` (≤767px) is now ONE wrapping flex line: title
  `flex: 1 1 auto; min-width: 0`, `justify-content: flex-end`, `order` for
  visual sequence. Flex line-breaking uses max-content sizes, so the radical
  box and the icon row stay on the title's line whenever they fit and wrap to
  their own right-flush line only when the title needs that width.
  Measured: 375px /萌 → 2 rows (heading 193px → 145px); 560px → all one line;
  375px /一鼓作氣 → title line 1, controls share line 2; 1280px unchanged.
- `.entry-copy-status` becomes an absolutely positioned pill under the icon
  row, visibility driven by `data-state="idle|ok|error"`. The node is never
  unmounted (an aria-live region must exist before its content changes), and
  out-of-flow satisfies the zero-layout-shift contract more directly than the
  reservation did. Background uses --moe-surface-alt; --moe-surface equals the
  entry card in dark mode and made the pill invisible.
- `.radical` is nowrap with a 0.15em column gap, so the 部首/筆畫 badge is one
  line again, and its stroke-order pencil joins the toolbar at a 44px touch
  target (legacy sized it 32x30).
- `row-gap: 0.5rem` gives clearance for ruby tone marks, which paint below
  h1.title's own box and otherwise graze a wrapped control line.

Tests updated to the new contract: `mobile: controls share the radical's line
when the heading still has room` (560px) is the guard for the reported
behaviour; the 333px test now asserts controls clear the title and the icon
row stays right-flush; the enlarged-64px-title test asserts both groups clear
the title rather than stacking; visual-invariants R8 (320/390px) asserts the
status region is out of flow, that nothing is reserved left of the first icon,
and that the longest failure message stays inside the viewport.

Also in this commit, because it is the same in-flight narrow-heading work this
checkout carried uncommitted and my layout is verified on top of it (see
5b1a7de on origin/fix/narrow-heading-wrap): the title ruby box in index.css
(e2e blocks r2-assets styles.css, without which length-3 yin+diao cannot share
a heading) and the moe-android diao centering.

Separately, plain `vp dev` gains a dev proxy for the paths the Worker owns
(`/api`, `/assets` → VITE_DEV_API_ORIGIN, default www.moedict.tw, with
`public/` winning via bypass). Without it those paths hit Vite's SPA fallback
and returned index.html, which surfaced as `取得 ASSET_BASE_URL 失敗:
SyntaxError: JSON.parse` and `MIME 型態「text/html」不是「text/css」`.
AGENTS.md documents both dev modes and the heading-layout invariant.

Verified: chromium e2e 259 passed, webkit-romanization 15, unit 2125,
integration 141, tsc -b --noEmit clean, vp lint clean, dark-contrast audit
0 unexpected findings.
Two long-lived sources of `git status` noise in the shared checkout, both now
resolved by deleting the artefacts and closing the gap that let them appear:

- 76MB of `fonts/TW-Kai-shard-{0..7}.ttf` at the repo root — the output of
  `scripts/build-tw-kai-shards.mjs --out-dir=fonts`, deterministic and
  checksum-verified against `data/assets/fonts/tw-kai-shards.manifest.json`,
  with the served copies already in R2 (`r2-assets.moedict.tw/fonts/...`). The
  existing rule stated the intent ("Never commit untracked font binaries into
  git history") but only matched `data/assets/fonts/`, so the build dir was a
  commit candidate for six days. Now `/fonts/` is ignored too.
- `docs/issue-triage/` plus `docs/{materialize,verify}-open-issues-dashboard.mjs`
  — a 2026-07 triage workbench for the g0v/moedict-webkit backlog. That backlog
  is down to one open issue (#219), `open-issues-dashboard.data.js` was a
  timestamped snapshot of the 39-issue state, and the verifier hardcodes
  `EXPECTED_LIVE_OPEN = 39` in seven assertions, so committing it would have
  landed a permanently failing check. Deleted, and the `!docs/issue-triage/**`
  negations added for it in cc0ec62 are removed rather than left pointing at
  nothing.
@audreyt
audreyt force-pushed the ship/2026-08-17-heading-layout branch from 6d4c108 to fec3ec9 Compare August 17, 2026 06:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the dictionary entry heading layout so that, on mobile widths, the radical chip and title control icons stay on the same flex line when space permits (and only wrap when the title truly needs the width). It also improves local development ergonomics by proxying Worker-owned paths when running plain Vite without the Cloudflare plugin.

Changes:

  • Reworks mobile .entry-heading layout to a single wrapping flex row (instead of pinned grid rows) and makes .entry-copy-status an out-of-flow pill controlled by data-state.
  • Adds/updates Playwright assertions guarding the “share line vs wrap” behavior and the out-of-flow status pill invariants.
  • Adds a dev-only proxy for /api/* and /assets/* in vp run dev (when the Worker isn’t mounted), with a public/ bypass.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
vite.config.ts Adds dev-only proxying for Worker-owned routes when running plain Vite; refactors proxy target to VITE_DEV_API_ORIGIN.
src/index.css Implements the mobile wrapping flex layout and converts copy-status to an absolute, out-of-flow pill; adds zhuyin box rules for e2e environment parity.
src/pages/DictionaryPage.tsx Adds data-state to the copy-status live region to drive CSS visibility state.
tests/e2e/dictionary.spec.ts Adds mobile geometry tests ensuring controls share the radical’s line when possible and wrap below long titles when not.
tests/e2e/visual-invariants.spec.ts Updates R8 invariants to assert out-of-flow status pill and no horizontal overflow at narrow widths.
AGENTS.md Documents the two dev modes and the intended mobile heading control layout invariants/tests.
.gitignore Updates docs ignore rules and adds /fonts/ to ignore generated TW-Kai shard output.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread vite.config.ts Outdated
audreyt and others added 2 commits August 17, 2026 14:27
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…nent

Follow-up to 3f7a56f (Copilot Autofix), keeping its `statSync` try/catch and
narrowing the remaining gaps in the same helper:

- AGENTS.md makes `tryDecodeURIComponent` (src/utils/dictionary-route.ts) the
  single decode path for request paths — "禁止裸呼 decodeURIComponent" — so the
  local try/catch is replaced by that call rather than duplicating the rule at a
  second site. Same behaviour, one definition.
- `new URL(req.url, …)` is guarded too: it is the other throw site in a hook
  where an exception fails the request instead of the lookup.
- Exported so the contract is testable. tests/unit/dev-proxy-public-bypass.test.ts
  pins what the review was actually about: `/assets/%`, `/assets/%zz`,
  `/api/%E0%A4%A` and `/%C0%80` answer false instead of throwing; a real
  `public/` file (and a query-stringed one) wins over the proxy; a directory
  does not; `..`, `%2e%2e` and NUL-byte paths refuse, with a positive control
  proving the traversal target exists on disk so those assertions fail closed
  for the right reason.

The test runs under `// @vitest-environment node` because vite.config.ts derives
`projectRoot` from `import.meta.url`, which is not a file: URL in the project's
happy-dom unit environment.

Smoke-verified against a live `vp dev`: `/assets/%` and `/assets/%zz` return 400
from the upstream Worker, `/api/%` returns 200 JSON, `/assets/images/icon.png`
is still served from public/, and the dev server stays up with no URIError.
@bestian

bestian commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@audreyt

請問這個PR需要協助檢閱和修訂嗎? 目前它和main有版本衝突conflict。
如果需要協助檢閱和修訂,可以用華文告知待檢閱的重點,以利接手嗎? 謝謝!

…conflict)

bestian 回報本 PR 與 main 衝突(issuecomment-5672694868)。解法:版面部分
以 main 為準,PR 只保留 main 還沒有的內容。

Layout 取 main(#173 已出貨,PR 的舊設計退場):
- src/index.css、src/pages/DictionaryPage.tsx、tests/e2e/dictionary.spec.ts、
  tests/e2e/visual-invariants.spec.ts 四檔直接取 main 版。PR 的 wrapping-flex
  單列(data-state pill、display:contents)與 main 的 narrow rail(grid 窄欄+
  8rem 預留+.entry-compact-title)互斥;#173 已合併且有 R8/e2e 鎖定,舊設計
  不留。
- AGENTS.md 的標題列版型 bullet 改寫為 narrow-rail 描述(原 wrapping-flex
  描述與出貨行為矛盾,一併修正)。

PR 保留(main 沒有,合併後 PR diff 只剩這四檔):
- vite.config.ts:Worker-owned 路徑 dev proxy+public/ bypass(含 Copilot
  review 的 tryDecodeURIComponent/statSync hardening)。
- tests/unit/dev-proxy-public-bypass.test.ts:上述契約測試。
- .gitignore:/fonts/ 構建產物忽略+issue-triage 否定清理。
- AGENTS.md:本機 dev 兩種模式文件。

驗證:tsc -b --noEmit clean;dev-proxy 5 tests、offline-api 11 tests 通過;
四個版面檔與 origin/main byte-identical(沿用 main CI 已驗證的 e2e)。
@audreyt audreyt changed the title Fix layout to keep title controls on the radical's line Dev proxy for Worker-owned paths in plain vp dev + font-build gitignore Sep 15, 2026
@audreyt

audreyt commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

@bestian 衝突已解決,本 PR 現在是 MERGEABLE 狀態,可檢閱了。以下用華文說明重點:

衝突原因:本 PR 原本的版面修改(手機版 wrapping-flex 單列)與之後合併的 #173(narrow rail:grid 窄欄+8rem 預留+短標題橫簇)是互斥設計,src/index.css 與 tests/e2e/dictionary.spec.ts 因此衝突。由於 #173 已出貨且有 R8/e2e 鎖定,這次以 main 為準,四個版面檔直接取 main 版,PR 標題與內文也已更新,不再是版面 PR。

待檢閱的重點(合併後 PR diff 只剩四檔):

  1. vite.config.ts —— plain vp dev 把 /api、/assets proxy 到正式站,public/ 真實檔案優先;重點看 servedFromPublicDir 的 fail-closed 路徑(壞編碼、遍歷、NUL)與 Copilot review 兩點是否都處理了。
  2. tests/unit/dev-proxy-public-bypass.test.ts —— 上述契約的新測試。
  3. .gitignore —— fonts/ 構建產物忽略與 issue-triage 否定清理。
  4. AGENTS.md —— dev 兩種模式文件,以及改寫為 narrow-rail 的版型 bullet 是否與出貨行為一致。

驗證:tsc -b --noEmit clean;dev-proxy 5 項、offline-api 11 項 unit 通過;版面四檔與 main byte-identical。謝謝!

Why this change is needed:

The public-directory bypass runs before Vite's remote Worker proxy. Although
ordinary dot segments are normalized by the WHATWG URL parser, an encoded path
separator such as `%2f` survives that parser and becomes a separator only after
percent-decoding. The previous implementation normalized the decoded absolute
path before checking for `..`, so `/api/%2e%2e%2fmanifest.json` collapsed to
`/manifest.json`. Because `public/manifest.json` is a real file, the helper
returned true, skipped the production proxy, and let Vite answer with its SPA
HTML fallback. This violated the documented fail-closed contract and recreated
the wrong-MIME/HTML-response failure that the dev proxy is intended to prevent.

The existing traversal assertions did not expose the gap: their ordinary dot
segments were already removed by `new URL()`, and the package.json positive
control only checked that a constructed string ended in the expected filename
instead of proving the file existed.

What changed:

- Reject NUL bytes and decoded path segments equal to `..` before calling
  `path.posix.normalize`, while recognizing both slash and backslash separators
  so the guard remains fail-closed on POSIX and Windows hosts.
- Preserve the existing post-normalization containment and guarded `statSync`
  checks as defense in depth.
- Add regressions for encoded-slash traversal under both `/api` and `/assets`,
  plus encoded-backslash traversal for Windows path semantics.
- Replace the ineffective string-suffix positive control with real `statSync`
  assertions for the repo-root package.json and public/manifest.json targets.

Verification:

- targeted dev-proxy unit suite: 5 tests passed
- TypeScript project check: passed
- Oxfmt check for both changed files: passed
- type-aware Oxlint for both changed files: passed
Why this is needed:

The cn-specific contrast fixture only needs to inject a synthetic alt field
into the API response for ~上訴. Its previous /api/~*.json route pattern also
captured follow-up dictionary requests triggered while rendering xref labels,
including ~非常上訴. The assertion could finish and close the page while one
of those unrelated route.fetch() callbacks was still awaiting a response. In
CI that race surfaced first as "Target page, context or browser has been
closed" and then, on retry, as "Response has been disposed". The same failure
reproduced on both the original job and its rerun.

The browser console message about r2-assets.test.local/js/es5-sham.js is not
the cause: that hostname is intentionally used by the E2E fixture for blocked
legacy remote assets, and the message also occurs in passing runs.

What changed:

- Restrict CN_API_PATTERN to the percent-encoded ~上訴 JSON endpoint that the
  helper is meant to modify.
- Continue accepting an optional query string and URL hex-case differences.
- Let xref-triggered /api/~*.json requests bypass the helper so no unrelated
  route callback remains in flight during page teardown.
- Leave the four layout files unchanged from origin/main.

Verification:

- The formerly failing dark-mode xref test passed 10/10 local repetitions with
  three Chromium workers.
- ./node_modules/.bin/tsc -b --noEmit completed cleanly.
- dev-proxy-public-bypass passed all 5 unit tests.
- offline-api passed all 11 unit tests.
- vp check reported 0 errors (with one pre-existing no-control-regex warning).
- src/index.css, src/pages/DictionaryPage.tsx,
  tests/e2e/dictionary.spec.ts, and tests/e2e/visual-invariants.spec.ts are
  byte-identical to origin/main.
@bestian

bestian commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@audreyt 我這邊查到了需要更動的部份,已於第
cf6fb09
變更中實作,請參考其說明

另外有一則e2e測試一直過不了,已於第
162c462
變更中修改測試使之通過

如果方便的話,請得便再double check一次好嗎?

如果這兩筆更動沒問題,即可merge了(目前被blocked的原因僅剩變更未簽章)

@audreyt
audreyt merged commit fcffd21 into main Sep 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants